Fix #19596: unit-parameter member conformance with signature by T-Gro · Pull Request #19615 · dotnet/fsharp

T-Gro · 2026-04-19T17:23:56Z

Fixes #19596

Stacked on #19609.

member M(()) produces ValReprInfo [[]] (empty arg group), but sig member M: unit -> unit parses to [[unit_arg]]. Both produce identical IL (.method instance void M()). The conformance checker rejected them because TotalArgCount differs (1 vs 2 including this).

Fix: Relax signature conformance matching for the unit-parameter case:

checkValInfo: allow empty impl group [] to match singleton sig group [_]
Overloaded matching: fallback using typeAEquivAux EraseAll + TotalArgCount diff=1 guard
Single-val [av],[fv] path: same relaxation

Safety:

IL verified identical via verifyILContains test
EraseAll matches the erasure mode used by valLinkageAEquiv in the primary matching path
TotalArgCount diff=1 guard restricts to unit-param case only
Existing E_StructConstructor01 conformance test still correctly fails
Both directions tested with consumer: sig+impl+consumer compiles correctly

- Release note for #19596 now correctly references PR #19615 (not #19609) - Add missing trailing newline to TypeTests.fs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot · 2026-04-21T08:10:40Z

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 66e6da3. Here's what was done for each conflict:

NicePrint.fs: Took base branch's cleaner Option.defaultValue/IsNullOrEmpty null handling
PrettyNaming.fs: Took base branch's public EscapeActivePatternCases and removed the redundant second escape application from my branch's private escapeActivePatternCaseNames
TypeTests.fs: Kept my HEAD tests (the unit-param conformance fix tests) and removed the Skip attribute from the roundtrip test since the issue is now fixed
TooltipTests.fs: Took base branch's removal of the infeasible backtick tooltip test

Build verified: 0 warnings/errors.

Copilot · 2026-04-28T11:08:48Z

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit e1bf547:

PrettyNaming.fsi: Made EscapeActivePatternCases internal (matching the base branch review fix)
TypeTests.fs: Kept my unit-param conformance tests; removed stray <<<<<<< HEAD marker left from the previous merge; kept roundtrip test enabled (no Skip)
TooltipTests.fs: Included all new method/property tagging tooltip tests from the base branch

Build: 0 warnings/errors.

T-Gro

Review: Fix #19596 — unit-parameter member conformance

The core fix in SignatureConformance.fs is sound for the overloaded case. The three-part approach — relaxed arg group arity in checkValInfo, min-guarded List.take, and type-equivalence fallback matching — correctly handles the M(()) vs M: unit → unit discrepancy.

Key concern: single-val path gap (@abonie's comment is correct)

The [av],[fv] path at line 689 still uses valuesPartiallyMatch which requires strict TotalArgCount equality. A non-overloaded member M(()) with sig member M: unit → unit will fail here (TotalArgCount 1 vs 2). The relaxed matching only applies to the multi-val _ -> branch.

This means the fix only works when M has overloads (so it enters the _ -> branch). A solo member M(()) = () paired with sig member M: unit → unit will still fail. This should be addressed — either with a type-equivalence fallback in the [av],[fv] path, or tracked as a follow-up issue.

Minor observations

NicePrint.fs change is a separate concern — the SRTP constraint dedup in layoutNonMemberVal is unrelated to unit-param conformance. Consider splitting into its own commit for clearer attribution.
FS0503 test documents a behavioral asymmetry — d.M() fails when impl is M(()) but succeeds when impl is M(), despite both having sig member M: unit → unit. The sig should define the API contract, so consumers shouldn’t see impl-level calling convention differences. This may be a pre-existing design constraint, but worth documenting in a comment or tracking separately.
Test coverage for the overloaded path is excellent — both directions, consumer access, IL verification, and negative cases are covered.

SignatureConformance.fs: when matching overloaded members, add relaxed fallback for unmatched pairs using type equivalence. This handles member M(()) (argInfos=[[]]) vs sig member M: unit->unit (argInfos=[[unit_arg]]) where types are equivalent but TotalArgCount differs. Also relax checkValInfo arg group check: empty impl group is compatible with singleton sig group for unit-parameter members. Tests: roundtrip test, handwritten sig+impl+consumer (both directions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ures Proves the conformance relaxation is safe at IL level — both member M(()) and member M() compile to '.method public hidebysig instance int32 M()', confirming the representation difference is compile-time only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Consumer cannot call d.M() when impl is M(()) with overloads (FS0503 expected) - Consumer can call d.M() when impl is M() with sig M: unit -> unit Covers both directions of sig↔impl conformance with consumer validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add type-equivalence fallback in [av],[fv] single-val path so non-overloaded member M(()) conforms to sig member M: unit -> unit (addresses abonie + T-Gro feedback) - Use idiomatic implGroup.IsEmpty && sigGroup.Length = 1 (T-Gro nit) - Add test for non-overloaded unit-param member conformance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ixes - Fix release notes PR number: #19609 -> #19615 - Prevent double-consume of impl vals in relaxed matching by using List.fold to track remaining available avs - Use obj.ReferenceEquals instead of System.Object.ReferenceEquals for consistency with codebase style - Add trailing newline to TypeTests.fs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-05-13T08:10:54Z

❗ Release notes required

✅ Found changes and release notes in following paths:

Change path Release notes path Description

src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

…ixes - Fix release notes PR number: #19609 -> #19615 - Prevent double-consume of impl vals in relaxed matching by using List.fold to track remaining available avs - Use obj.ReferenceEquals instead of System.Object.ReferenceEquals for consistency with codebase style - Add trailing newline to TypeTests.fs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

abonie

🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.

abonie · 2026-05-18T09:47:14Z

                  let res = 
-                      (implArgInfos, sigArgInfos) ||> List.forall2 (List.forall2 (fun implArgInfo sigArgInfo -> 
+                      (implArgInfos, sigArgInfos) ||> List.forall2 (fun (implGroup: ArgReprInfo list) (sigGroup: ArgReprInfo list) ->
+                          // When impl group is empty (unit param like member M(())), skip arg-level checks


[Type System] When implGroup is empty we skip arg-level propagation entirely. That drops the sig unit parameter name/attributes (and InlineIfLambda checks) and leaves ValReprInfo empty, so callers can still hit the FS0503 case where d.M() is rejected despite the sig. Consider synthesizing a unit ArgReprInfo from the sig (replace the empty impl group with the sig group) so SetValReprInfo reflects the signature.

T-Gro requested a review from a team as a code owner April 19, 2026 17:23

github-project-automation Bot added this to F# Compiler and Tooling Apr 19, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Apr 19, 2026

T-Gro changed the base branch from sig-roundtrip-sweep to main April 19, 2026 17:30

T-Gro changed the base branch from main to sig-roundtrip-sweep April 19, 2026 17:37

T-Gro force-pushed the fix/unit-member-conformance branch from e8e6743 to e4bc6ae Compare April 20, 2026 13:50

T-Gro force-pushed the sig-roundtrip-sweep branch 2 times, most recently from 4b5e839 to 3608686 Compare April 20, 2026 18:38

Copilot started work on behalf of T-Gro April 21, 2026 07:50 View session

Copilot finished work on behalf of T-Gro April 21, 2026 08:12

T-Gro force-pushed the sig-roundtrip-sweep branch from 3608686 to 8923561 Compare April 28, 2026 09:15

T-Gro requested a review from abonie April 28, 2026 09:56

Copilot started work on behalf of T-Gro April 28, 2026 10:54 View session

Copilot finished work on behalf of T-Gro April 28, 2026 11:11

abonie reviewed Apr 28, 2026

View reviewed changes

Comment thread tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs

This comment was marked as off-topic.

Sign in to view

T-Gro added AI-reviewed PR reviewed by AI review council and removed AI-reviewed PR reviewed by AI review council labels May 7, 2026

T-Gro commented May 12, 2026

View reviewed changes

Comment thread src/Compiler/Checking/SignatureConformance.fs

Comment thread src/Compiler/Checking/SignatureConformance.fs Outdated

Comment thread src/Compiler/Checking/NicePrint.fs Outdated

T-Gro added the AI-reviewed PR reviewed by AI review council label May 12, 2026

T-Gro and others added 5 commits May 13, 2026 09:58

Update release notes: add #19596 fix, fix PR numbers

4e7a8b1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

T-Gro force-pushed the fix/unit-member-conformance branch from e1bf547 to c94466e Compare May 13, 2026 08:00

T-Gro changed the base branch from sig-roundtrip-sweep to main May 13, 2026 08:00

T-Gro force-pushed the fix/unit-member-conformance branch from 0f7bbfe to 7ac3c70 Compare May 13, 2026 14:40

T-Gro requested a review from abonie May 14, 2026 09:23

abonie reviewed May 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #19596: unit-parameter member conformance with signature#19615

Fix #19596: unit-parameter member conformance with signature#19615
T-Gro wants to merge 6 commits into
mainfrom
fix/unit-member-conformance

T-Gro commented Apr 19, 2026 •

edited

Loading

Uh oh!

Copilot AI commented Apr 21, 2026

Uh oh!

Copilot AI commented Apr 28, 2026

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

T-Gro left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

abonie left a comment

Uh oh!

abonie May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

T-Gro commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI commented Apr 21, 2026

Uh oh!

Copilot AI commented Apr 28, 2026

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

T-Gro left a comment

Choose a reason for hiding this comment

Review: Fix #19596 — unit-parameter member conformance

Key concern: single-val path gap (@abonie's comment is correct)

Minor observations

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 13, 2026

❗ Release notes required

Uh oh!

abonie left a comment

Choose a reason for hiding this comment

Uh oh!

abonie May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

T-Gro commented Apr 19, 2026 •

edited

Loading